-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[4.0] [com_privacy] Fix default value for not nullable datetime columns #26461
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[4.0] [com_privacy] Fix default value for not nullable datetime columns #26461
Conversation
…-default-com-privacy
This reverts commit 1e76ce3.
administrator/components/com_admin/sql/updates/postgresql/4.0.0-2019-09-26.sql
Outdated
Show resolved
Hide resolved
Co-Authored-By: Quy <[email protected]>
…-default-com-privacy
|
|
||
| if (!$this->confirm_token_created_at) | ||
| { | ||
| $this->confirm_token_created_at = $date->toSql(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this in real life? doesn't this need to be nullable until the confirm token is actually created?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question. To be honest, I don’t remember right now why I‘ve done it that way. Will check later today or tomorrow and if necessary correct that. Any hint where in code this token is created would be very welcome.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Found where the token is set. Will check later what to do here at this place.
|
Closing in favour of PR #26561 . |
Pull Request for Issue #24535 (part).
Summary of Changes
This PR fixes all datetime columns of com_fields tables so there will not be any
Invalid value '0000-00-00 00:00:00' for datetimeerror anymore on MySQL 5.7 or later when strict mode is enabled, and thedatetime(MySQL) ortimestamp without time zone(PostgreSQL) columns will allowNULLvalues wherever useful/possible.Following is changed:
#__privacy_requests's columnsrequested_atandconfirm_token_created_atwill be treated in the same way as e.g. columnscreatedandmodifiedof the#__bannerstable in PR [4.0] [com_banners] Finish nullable and fix default values for not nullable datetime columns #26372 , i.e. they will get no default value.#__privacy_consents's columncreated.The default value is only used when inserting new records without specifying values for that particular column. Not having a default will enforce to insert new records with values for these columns being provided and throw an SQL error if some of these values is not specified, i.e. such errors will not be hidden anymore.
Old data will be updated as little as possible. The
requested_atandcreatedcolumns will be not touched at all. We can assume that for core components there is no data with values '0000-00-00 00:00:00' on MySQL or '1970-01-01 00:00:00' on PostgreSQL, and data created by 3rd party components should not be modified. Theconfirm_token_created_atcolumn will be set to the value of therequested_atcolumn only has value '0000-00-00 00:00:00' on MySQL or '1970-01-01 00:00:00' on '1970-01-01 00:00:00'.Testing Instructions
Testers please report back the database kind (MySQL or PostgreSQL) on which you have tested.
If you have both MySQL and PostgreSQL, please test on both if possible.
Test 1: New installation
configuration.phpand delete all Joomla database tables in PhpMyAdmin or PhpPgAdmin (depending on your database type).datetime/timestamp without timezonecolumns.Result: See section "Expected result" below.
Test 2: Update sql script
installation/sql/mysql/joomla.sqlinto the SQL command window but don't execute the commands yet:This switches off strict mode to the SQL will run on MySQL 5.7 or later.
administrator/components/com_admin/sql/updates/mysql/4.0.0-2019-09-26.sqloradministrator/components/com_admin/sql/updates/postgresql/4.0.0-2019-09-26.sql(depending on your database type) into the SQL command window, in case of MySQL below the previously pasted commands, but don't execute the commands yet.#__by your database prefix in the SQL statements pasted before in the SQL input window.datetime/timestamp without timezonecolumns.Result: See section "Expected result" below.
Expected result
New Installation
The Privacy Component works as well as without this PR. In a MySQL database there are no columns of data type
datetimehaving value '0000-00-00 00:00:00' in tables#__privacy_requestsand#__privacy_consents, and there is no invalid default value anymore in MySQL >= 5.7 with strict mode on. On PostgreSQL there are no columns of data typetimestamp without time zonehaving value '0000-00-00 00:00:00' in these tables.Update sql script
The statements are processed without error. The expected result is the same as for a new installation.
Actual result
New Installation
On MySQL same as expected, but the default value of database columns of data type
datetimehaving value '0000-00-00 00:00:00' in tables#__privacy_requestsand#__privacy_consentsis invalid in MySQL >= 5.7 with strict mode on, and there might be values '0000-00-00 00:00:00'. On postgreSQL same as expected, but there might be values '1970-01-01 00:00:00' in the datetime columns of tables#__privacy_requestsand#__privacy_consents.Documentation Changes Required
Maybe core developer docs and extension developer docs should be updated to encourage them not to use '0000-00-00 00:00:00' on MySQL anymore but use real NULL and not abuse '1970-01-01 00:00:00' on PostgreSQL as a speudo null date anymore and use real NULL values also there.